fix(render): retry timed-out browser initialization once - #3801

Read the full files at head, not just the diff, and verified the Why section empirically rather than by reading the pattern lists.

Strengths

• The classifier addition lands in the right place. PROTOCOL_TIMEOUT_PATTERNS is evaluated before TRANSIENT_BROWSER_ERROR_PATTERNS (packages/engine/src/services/captureFailure.ts:153-155), so the new signature can't be swallowed by the transient branch and reach the same-worker-count retry instead. And the cancelled guard still precedes the new block (packages/producer/src/services/renderOrchestrator.ts:1151), so an aborted render can't burn the initialization retry — the parameterized cancellation test pins that for both messages.
• The new branch is tightly scoped: initializationRetriesUsed === 0 plus the kind plus an exact-signature regex, so it can't drift into a general zero-progress retry. The comment says exactly that.
• The "Why" claim checks out. I ran both versions of the classifier against the three real message shapes. At base 3a7fcd10e036ff0f5837821bb1143ca529934fae, Network.enable timed out classified as authoring with isFatalCaptureFailure true; at head it is protocol_timeout and false. "Classified as authoring" is accurate.

important — the reclassification also changes peer-abort semantics in the engine, and nothing states or tests it

isFatalCaptureFailure (captureFailure.ts:180) excludes protocol_timeout, and the parallel coordinator uses exactly that predicate to decide whether one worker's failure kills the others (packages/engine/src/services/parallelCoordinator.ts:1051-1055):

const onFailure = (failure: CaptureFailure): void => {
  if (firstFatalFailure || !isFatalCaptureFailure(failure)) return;
  firstFatalFailure = failure;
  peerController.abort(failure);
};

Before this PR a worker's Network.enable timeout was authoring → fatal → every peer worker was aborted. After it, the failure is non-fatal, so peers are no longer aborted and run to completion. Concretely, with 4 workers where worker 0's session never initializes and 1-3 are healthy:

1. The failing attempt now takes as long as the slowest healthy worker instead of failing fast. protocolTimeout defaults to 300_000 ms (packages/engine/src/config.ts:282), so the stuck worker alone can sit 5 minutes.
2. Because those peers now write frames, madeProgress is true — so the new branch at :1215 does not fire (it requires !madeProgress). Recovery falls through to the pre-existing halving retry at :1249. The new branch's real scope is therefore "no worker made any progress at all."

I think (1)+(2) are probably a net improvement — progress is preserved, so the retry redoes less — but this is a behavior change in a different package than the PR's stated scope, it changes a publicly exported predicate (packages/engine/src/index.ts:133-135), and no test covers it. Worth a line in the body and a coordinator test pinning whichever semantics you intend. Related: because (2) routes the partial case to the halving retry, which requires currentWorkers > 1, a single-worker render with partial progress still throws exactly as before.

important — the bound test asserts a constant that does not govern the new path

In renderOrchestrator.test.ts, it.each(["Session closed", "Network.enable timed out"])("bounds repeated %s failures") asserts 1 + MAX_TRANSIENT_CAPTURE_RETRIES. That is the right constant for Session closed. For Network.enable timed out the governing bound is the new hardcoded initializationRetriesUsed === 0, not MAX_TRANSIENT_CAPTURE_RETRIES. Traced: attempt 1 fails → init retry fires and halves 1 → 1; attempt 2 fails → init retry spent, transient branch skipped (wrong kind), so currentWorkers <= 1 throws. Two calls. It passes only because MAX_TRANSIENT_CAPTURE_RETRIES is 1 (renderOrchestrator.ts:944) and coincidentally equals the initialization budget.

Raise that constant to 2 and the Network.enable case fails at 2-vs-3 while pointing the reader straight at the transient path, which isn't the code that bounded it. Assert the literal 2, or add a MAX_INITIALIZATION_RETRIES constant and assert against that.

nit — once the initialization retry is spent, a second Network.enable failure lands on the !madeProgress warning at :1240: "composition is likely structurally broken — not retrying." That text predates this PR, but the PR makes browser-startup timeouts a routine visitor to it. Including failure.kind there would save the next person a detour. Adjacent code, so entirely your call.

Audited / Trusting

• Audited: captureFailure.ts and the executeDiskCaptureWithAdaptiveRetry loop end-to-end at head. Enumerated every consumer of the failure taxonomy across all 2219 TS files at this SHA (isFatalCaptureFailure, classifyCaptureFailure, isRecoverableParallelCaptureError).
• Checked and NOT affected: packages/producer/src/services/distributed/renderChunk.ts:274 — shouldRetryChunkCaptureWithScreenshot uses the kind only to exclude cancelled/memory_exhaustion, and this signature matches none of its beginFrame regexes, so the distributed screenshot fallback is unchanged. captureStageError.ts:9 only propagates the kind.

Head reviewed: d0ee207b06432edc48d3d0879c4cb4ca387cd0fd

Verdict: APPROVE
Reasoning: The recovery logic is correct and narrowly gated, cancellation is preserved, and the body's central claim about the old classification is verifiable and true. The two important items are an unstated side effect and a test coupled to the wrong constant — neither breaks the path this fixes, but both are worth resolving while the context is fresh.

— Rames Jusso